-
Notifications
You must be signed in to change notification settings - Fork 220
feat: cache desired state #2831
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Chris Laprun <[email protected]>
@metacosm what is the use case for this? Desired pojos are usually extremely quick to create, since values comes from caches. |
This isn't always true, though, in particular when the desired state needs to deal with external systems. Another aspect to this, also when dealing with external systems, is that it can be difficult to ensure idempotency of the desired result, therefore repeated calls to |
External cache states should be cached in event sources. That is the pattern by default.
Yes, but this is the cases with all our caches, and in general is not handled neither in go. |
I'm not sure I follow you. There is no pattern for this in JOSDK since there are no specific event sources for external resources.
That's actually a good point and that's something JOSDK should probably do out of the box: the desired state should be put in the context once it's been computed once and should be retrieved from there all the time instead of being recomputed. |
Why is it recomputed? |
The PollingEventSource, PerResourcePollingEventSource, CachingInboundEventSource are for this purpose. |
|
We don't have examples for how to use this in the context of dependents (not even sure how I would do it, I haven't looked), is what I mean. The context for this is that the |
Lines 111 to 123 in 127e87d
In my sql we show how to use, the fetch method is generic, user can fill in its logic. Also you can use arbitrary event source referenced by name from DR. |
This isn't the use case I'm talking about, though. In the case of the Schema example, the dependent itself is external. The use case I'm talking about and that isn't covered is where a dependent is a kubernetes dependent (in this case a Secret) so extends |
I see, but in this case it is not just a an additional event source for that 3rd party service to fetch required information? (then user can access it from desired method through context) |
@@ -33,6 +33,7 @@ public abstract class AbstractDependentResource<R, P extends HasMetadata> | |||
private final boolean updatable = this instanceof Updater; | |||
private final boolean deletable = this instanceof Deleter; | |||
private final DependentResourceReconciler<R, P> dependentResourceReconciler; | |||
private final ThreadLocal<R> desiredCache = new ThreadLocal<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if using ThreadLocal is the best for this purpose, if some later call the dependent from reconcile method it will happen on different thread so desired won't be available
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'm not sure either if that's the proper way to do it…
Signed-off-by: Chris Laprun [email protected]